Skip to content

feat(home): add service visibility customization#3091

Merged
biplab1 merged 32 commits intoopenMF:developmentfrom
Aditya002500:devy
Feb 12, 2026
Merged

feat(home): add service visibility customization#3091
biplab1 merged 32 commits intoopenMF:developmentfrom
Aditya002500:devy

Conversation

@Aditya002500
Copy link
Contributor

@Aditya002500 Aditya002500 commented Jan 22, 2026

Fixes - MM-505

Added Icon besides Service

|

WhatsApp.Video.2026-02-10.at.22.07.42.mp4

|

Please make sure these boxes are checked before submitting your pull request - thanks!

  • Run the static analysis check ./gradlew check or ci-prepush.sh to make sure you didn't break anything

  • If you have multiple commits please combine them into one commit by squashing them.

Summary by CodeRabbit

Release Notes

  • New Features
    • Added the ability to customize which services appear on the home screen through a new edit mode.
    • Users can toggle edit mode to select and deselect services, with visual checkmarks indicating selected items.
    • Added helpful hint text when no services are available.
    • New UI icons added for improved visual design.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Jan 22, 2026

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review
📝 Walkthrough

Walkthrough

This pull request introduces service selection functionality to the home screen, enabling users to toggle edit mode, selectively display services, and persist their preferences. Changes span the data persistence layer (DataSource and Repository), UI components (HomeScreen with edit and selection UI), state management (HomeViewModel with new actions and computed state), and design system (new icon resources).

Changes

Cohort / File(s) Summary
Data Persistence Layer
core/datastore/.../UserPreferencesDataSource.kt, core/datastore/.../UserPreferencesRepository.kt, core/datastore/.../UserPreferencesRepositoryImpl.kt, core/datastore/.../model/AppSettings.kt
Added new public API methods to manage selected services: setSelectedServices(), saveSelectedServicesDirectly(), getSelectedServicesDirectly() in DataSource; corresponding interface and implementation in Repository; and new selectedServices property in AppSettings data class for serialization.
Design System
core/designsystem/.../icon/MifosIcons.kt
Added three new public icon properties: GridApps, CheckCircle1, and Pencil for UI support.
Home Feature UI
feature/home/.../HomeScreen.kt
Refactored ServiceBox to accept visibleItems, isEditMode, selectedServices, and onServiceClick instead of items and onAction. Enhanced ServiceItemCard with isSelected and isEditMode parameters; added adaptive styling for edit mode, checkmark display, and no-services hint text.
Home Feature State Management
feature/home/.../HomeViewModel.kt
Extended HomeState with visibleItems, selectedServices, and isEditMode. Added new actions ToggleEditMode and ToggleServiceSelection. Implemented service loading, selection toggling, and persistence workflows with cancellation handling.
Home Feature Resources
feature/home/.../composeResources/values/strings.xml
Added three new string resources: feature_home_edit_services, feature_home_selected, and feature_home_no_services_hint.

Sequence Diagram

sequenceDiagram
    participant User
    participant HomeScreen
    participant HomeViewModel
    participant UserPreferencesRepository
    participant UserPreferencesDataSource

    User->>HomeScreen: Toggle Edit Mode
    HomeScreen->>HomeViewModel: ToggleEditMode action
    HomeViewModel->>HomeViewModel: Update isEditMode state
    
    User->>HomeScreen: Select/Deselect Service
    HomeScreen->>HomeViewModel: ToggleServiceSelection(route) action
    HomeViewModel->>HomeViewModel: Toggle service in selectedServices
    HomeViewModel->>HomeViewModel: Compute new visibleItems
    
    User->>HomeScreen: Confirm Service Selection (Exit Edit)
    HomeScreen->>HomeViewModel: ToggleEditMode action
    HomeViewModel->>UserPreferencesRepository: setSelectedServices(selectedServices)
    UserPreferencesRepository->>UserPreferencesDataSource: setSelectedServices(selectedServices)
    UserPreferencesDataSource->>UserPreferencesDataSource: Save to preferences
    UserPreferencesDataSource-->>UserPreferencesRepository: Success
    UserPreferencesRepository-->>HomeViewModel: Success
    HomeViewModel->>HomeViewModel: Update isEditMode = false
    HomeViewModel-->>HomeScreen: Updated state
    HomeScreen->>User: Display updated services
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Suggested reviewers

  • revanthkumarJ

Poem

🐰 Hops with glee through the changes bright,
Services now dance in edit's light,
Check and uncheck with a tap so fine,
Saved to preferences, a perfect design!

🚥 Pre-merge checks | ✅ 2 | ❌ 1
❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 21.74% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title 'feat(home): add service visibility customization' clearly and directly summarizes the primary change—adding the ability to customize which services are visible on the home screen.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@Aditya002500 Aditya002500 marked this pull request as ready for review January 29, 2026 18:14
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 3

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
feature/home/src/commonMain/kotlin/org/mifos/mobile/feature/home/HomeScreen.kt (1)

12-28: Add the missing clickable import to prevent a compile error.

Modifier.clickable is used at lines 254 and 344 but the corresponding import from androidx.compose.foundation is missing.

🐛 Add the missing import
 import androidx.compose.foundation.Image
+import androidx.compose.foundation.clickable
 import androidx.compose.foundation.border
🤖 Fix all issues with AI agents
In
`@core/datastore/src/commonMain/kotlin/org/mifos/mobile/core/datastore/UserPreferencesDataSource.kt`:
- Around line 239-253: The direct getters/setters currently use
SELECTED_SERVICES_KEY and diverge from the APP_SETTINGS path used by
setSelectedServices, causing stale state; change saveSelectedServicesDirectly
and getSelectedServicesDirectly to read/write the same SettingsPreference via
settings.getSettingsPreference() and settings.putSettingsPreference(...) (i.e.,
build a newPreference = settings.getSettingsPreference().copy(selectedServices =
services) when saving) and ensure they also update _settingsInfo.value to the
new preference so all callers share a single source of truth; reference
setSelectedServices, saveSelectedServicesDirectly, getSelectedServicesDirectly,
_settingsInfo, settings.getSettingsPreference and settings.putSettingsPreference
when making the change.

In
`@feature/home/src/commonMain/kotlin/org/mifos/mobile/feature/home/HomeScreen.kt`:
- Around line 370-373: Replace the hard-coded contentDescription "Selected" in
the Icon usage (the Icon with imageVector = MifosIcons.CheckCircle1 inside
HomeScreen.kt) with a localized stringResource; add a new string key
feature_home_selected to
feature/home/src/commonMain/composeResources/values/strings.xml (and localized
variants) and reference it via stringResource(Res.string.feature_home_selected)
for accessibility/i18n.
- Around line 248-251: The Icon's contentDescription is hard-coded; create a
localized string resource named "feature_home_edit_services" and replace the
literal "Edit services" with a lookup to that resource so the Icon uses the
localized text based on isEditMode (i.e., keep the Icon call using imageVector =
if (isEditMode) MifosIcons.Edit else MifosIcons.GridApps but set
contentDescription to the localized string obtained from resources), ensuring
the same resource is used for accessibility in HomeScreen.
🧹 Nitpick comments (3)
core/designsystem/src/commonMain/kotlin/org/mifos/mobile/core/designsystem/icon/MifosIcons.kt (1)

270-272: Prefer descriptive aliases over CheckCircle1 / Pencil.

These overlap existing CheckCircle and Edit aliases. Consider renaming to clarify intent (or reuse the existing names) to avoid confusion for consumers.

♻️ Suggested renames (update call sites accordingly)
-    val CheckCircle1 = FluentIcons.Filled.CheckmarkCircle
-    val Pencil = Icons.Filled.Edit
+    val CheckCircleFilled = FluentIcons.Filled.CheckmarkCircle
+    val EditFilled = Icons.Filled.Edit
core/datastore/src/commonMain/kotlin/org/mifos/mobile/core/datastore/UserPreferencesDataSource.kt (1)

35-35: Consider refactoring instead of a broad suppression (optional).

If feasible, splitting responsibilities would avoid the TooManyFunctions suppression and improve cohesion.

feature/home/src/commonMain/kotlin/org/mifos/mobile/feature/home/HomeScreen.kt (1)

294-295: Consider an empty-state when no services are selected.

When selectedServices is empty, the non-edit view renders nothing. A lightweight empty-state or fallback could prevent a blank UI.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
core/datastore/src/commonMain/kotlin/org/mifos/mobile/core/datastore/di/PreferenceModule.kt (1)

20-28: Conflicting Settings bindings in Koin modules must be resolved.

Two different scopes define Settings:

  • PreferencesModule (core/datastore): single<Settings>
  • PasscodePreferenceModule (libs/mifos-passcode): factory<Settings>

Both modules are loaded together in the production app via KoinModules.kt, causing Koin to fail or override bindings. Align both to use the same scope (recommend single<Settings> for shared application state), or refactor to prevent both from being loaded simultaneously.

🤖 Fix all issues with AI agents
In
`@feature/home/src/commonMain/kotlin/org/mifos/mobile/feature/home/HomeScreen.kt`:
- Around line 241-258: The Icon currently uses Modifier.size(16.dp).clickable {
toggleEditMode() } which yields a 16.dp touch target; replace this raw Icon
click handling with a Material IconButton to provide the standard 48dp
accessible touch target: wrap the Icon in an IconButton (or use IconButton
composable) and move the toggleEditMode() click handler to the IconButton's
onClick, keeping the same imageVector selection (if (isEditMode) MifosIcons.Edit
else MifosIcons.GridApps), contentDescription, tint, and size for the visual
icon while allowing the IconButton to ensure proper padding and accessibility in
the Row where Text and Icon are defined.

@Aditya002500 Aditya002500 changed the title Devy Added services that can be selected by the customer for being hidden or displayed in order to have only the options selected by the customer Jan 29, 2026
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 Fix all issues with AI agents
In
`@feature/home/src/commonMain/kotlin/org/mifos/mobile/feature/home/HomeScreen.kt`:
- Around line 242-259: The accessibility label currently always says "Edit
services" even when isEditMode is true; change the Icon's contentDescription to
be conditional on isEditMode (e.g., use a "Done/Save services" or "Exit edit
mode" string resource when isEditMode is true, and "Edit services" when false)
so the IconButton(toggleEditMode) accurately describes the action; update the
string resource keys if needed and reference isEditMode, toggleEditMode(),
IconButton and Icon in HomeScreen.kt when making the change.

Copy link
Contributor

@amanna13 amanna13 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey @Aditya002500 ! I have left few comments here. Please have a look

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 Fix all issues with AI agents
In
`@feature/home/src/commonMain/kotlin/org/mifos/mobile/feature/home/HomeScreen.kt`:
- Around line 147-151: toggleEditMode currently calls the blocking, non-suspend
preferencesRepository.saveSelectedServices(...) on the main thread; wrap that
call in a coroutine launched on a background dispatcher (e.g., Dispatchers.IO or
an injected/background CoroutineScope) so the synchronous SharedPreferences
write runs off the UI thread, while keeping any UI state updates (toggling
isEditMode) on the main/Main thread; locate the toggleEditMode function and
replace the direct call to
preferencesRepository.saveSelectedServices(selectedServices) with a coroutine
launch that calls that method on an IO/background scope and optionally handles
errors.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (3)
core/datastore/src/commonMain/kotlin/org/mifos/mobile/core/datastore/UserPreferencesDataSource.kt (1)

161-171: ⚠️ Potential issue | 🟡 Minor

Good fix to sync in-memory _userInfo on clear, but SELECTED_SERVICES_KEY is not cleared.

Adding _userInfo.value = UserData.DEFAULT (Line 164) is a solid fix to keep in-memory state consistent. However, clearInfo doesn't remove SELECTED_SERVICES_KEY — so a logged-out user's service selection persists across sessions. If the dual-storage path is consolidated into AppSettings (per comments above), this is automatically handled by the existing cleared copy on Line 165. Otherwise, add settings.remove(SELECTED_SERVICES_KEY).

feature/home/src/commonMain/kotlin/org/mifos/mobile/feature/home/HomeScreen.kt (2)

343-392: ⚠️ Potential issue | 🟡 Minor

Tap target excludes the service label text.

The clickable modifier is on the inner Box (Line 349) which wraps only the icon image. The Text label (Line 386) sits outside that Box, so tapping the label does nothing. Move the clickable to the outer Column (or wrap both in a single clickable surface) for a more intuitive hit area.

Proposed fix
     Column(
-        modifier = modifier.padding(vertical = KptTheme.spacing.sm),
+        modifier = modifier
+            .padding(vertical = KptTheme.spacing.sm)
+            .clickable { onClick() },
         verticalArrangement = Arrangement.spacedBy(KptTheme.spacing.sm),
         horizontalAlignment = Alignment.CenterHorizontally,
     ) {
         Box(
-            modifier = Modifier.clickable { onClick() },
+            modifier = Modifier,
         ) {

443-456: ⚠️ Potential issue | 🟠 Major

@Preview will crash — koinInject() requires a Koin context.

HomeContent now calls koinInject<UserPreferencesRepository>() (Line 140), but @Preview composables don't have a Koin dependency graph initialized. This will throw at preview render time.

This is another reason to move the repository interaction into HomeViewModel (as suggested above) — the preview only needs a HomeState, not a live DI container.

🤖 Fix all issues with AI agents
In
`@feature/home/src/commonMain/kotlin/org/mifos/mobile/feature/home/HomeScreen.kt`:
- Around line 299-300: When not in edit mode the filtered displayItems can be
empty (selectedServices empty) leaving users without guidance; update the
HomeScreen UI logic to detect when displayItems.isEmpty() and render an
empty-state placeholder message or composable (e.g., "Tap the grid icon to add
services") instead of the grid rows. Locate the displayItems/rows calculation
and the composable that renders chunked rows (references: displayItems, rows,
columnCount, isEditMode, selectedServices) and add a conditional branch to show
the placeholder when displayItems.isEmpty() and !isEditMode.
- Around line 150-158: The toggleEditMode currently launches a coroutine to call
preferencesRepository.saveSelectedServices but flips isEditMode immediately, so
save failures are ignored; change toggleEditMode so that when isEditMode is true
you perform the save inside scope.launch and only set isEditMode = false after a
successful save, catching exceptions from
preferencesRepository.saveSelectedServices (use try/catch or runCatching) and
handle failures by logging and surfacing an error to the user (e.g., snackbar or
error state) instead of toggling off; keep the immediate toggle behavior when
entering edit mode (when isEditMode is false) outside the save path. Use the
existing scope, toggleEditMode, isEditMode and
preferencesRepository.saveSelectedServices symbols to locate and update the
logic.
🧹 Nitpick comments (3)
core/datastore/src/commonMain/kotlin/org/mifos/mobile/core/datastore/UserPreferencesDataSource.kt (1)

35-35: Consider splitting UserPreferencesDataSource rather than suppressing the lint.

The class is growing (20+ public methods). A future refactor could extract service-selection concerns into a dedicated data source (e.g., ServiceSelectionDataSource), keeping each class focused.

feature/home/src/commonMain/kotlin/org/mifos/mobile/feature/home/HomeScreen.kt (2)

140-158: Service selection state is managed in the Composable, bypassing the ViewModel/MVI pattern.

The rest of HomeScreen follows a clean MVI pattern: HomeViewModelHomeStateHomeActionHomeEvent. But service selection is handled entirely in the Composable via koinInject(), local mutableStateOf, and a rememberCoroutineScope. This creates two separate state management approaches in the same screen.

Consider moving selectedServices, isEditMode, and the toggle/persistence logic into HomeViewModel and exposing them through HomeState/HomeAction. This would:

  • Keep a single source of truth for screen state
  • Make the logic testable without Compose
  • Align with the existing architecture

380-380: Hardcoded 4.dp — use a design token for consistency.

The rest of the file uses DesignToken and KptTheme spacing values. Replace 4.dp with the appropriate token (e.g., KptTheme.spacing.xs or DesignToken.spacing.extraSmall).

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
feature/home/src/commonMain/kotlin/org/mifos/mobile/feature/home/HomeScreen.kt (1)

343-392: ⚠️ Potential issue | 🟡 Minor

Service text label is not part of the tap target.

The clickable modifier is on the inner Box (Line 349) that wraps only the icon image. The Text label (Line 386) sits outside this Box in the parent Column, so tapping the service name does nothing. Users will likely expect the entire card (icon + label) to be tappable.

Move the clickable modifier up to the Column (or wrap the whole card in a single clickable surface):

Proposed fix
     Column(
-        modifier = modifier.padding(vertical = KptTheme.spacing.sm),
+        modifier = modifier
+            .clickable { onClick() }
+            .padding(vertical = KptTheme.spacing.sm),
         verticalArrangement = Arrangement.spacedBy(KptTheme.spacing.sm),
         horizontalAlignment = Alignment.CenterHorizontally,
     ) {
         Box(
-            modifier = Modifier.clickable { onClick() },
+            modifier = Modifier,
         ) {
🤖 Fix all issues with AI agents
In
`@feature/home/src/commonMain/kotlin/org/mifos/mobile/feature/home/HomeScreen.kt`:
- Around line 140-158: Move the service-selection state and persistence out of
the composable into HomeViewModel: stop calling koinInject() in
HomeScreen/HomeContent, inject UserPreferencesRepository into HomeViewModel
instead, expose selectedServices and isEditMode as StateFlow (or
MutableStateFlow) and provide a toggleEditMode() method that handles loading,
mutating and saving via coroutine scope inside the ViewModel; update
HomeScreen/HomeContent to consume those StateFlows and call
viewModel.toggleEditMode(), ensuring previews/tests can construct the composable
without Koin by passing mocked/empty state flows or a preview HomeViewModel.
🧹 Nitpick comments (1)
feature/home/src/commonMain/kotlin/org/mifos/mobile/feature/home/HomeScreen.kt (1)

378-381: Minor: prefer design token over hard-coded 4.dp.

Line 380 uses a literal 4.dp while the rest of the file consistently uses DesignToken.* or KptTheme.spacing.*. Consider using the appropriate token for consistency.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
feature/home/src/commonMain/kotlin/org/mifos/mobile/feature/home/HomeScreen.kt (1)

425-438: ⚠️ Potential issue | 🟡 Minor

Preview shows empty-state hint instead of service cards.

With the new defaults (selectedServices = emptySet(), isEditMode = false), the preview will render the "Tap the grid icon to add services" hint instead of actual service cards, since displayItems will be empty. Consider populating selectedServices so the preview is representative of normal usage.

Suggested adjustment
         HomeContent(
             state = HomeState(
                 dialogState = null,
                 items = serviceCards,
                 uiState = HomeScreenState.Success,
+                selectedServices = serviceCards.map { it.route }.toSet(),
             ),
             onAction = {},
             modifier = Modifier,
         )
🤖 Fix all issues with AI agents
In
`@feature/home/src/commonMain/kotlin/org/mifos/mobile/feature/home/HomeViewModel.kt`:
- Around line 248-264: handleToggleEditMode currently catches Exception which
swallows CancellationException and also silently discards persisted selections
on save failure; change the try/catch to rethrow CancellationException (or only
catch specific save-related exceptions) around the call to
userPreferencesRepositoryImpl.saveSelectedServices(state.selectedServices), and
on any non-cancellation save failure restore the UI selection by resetting
state.selectedServices to the last persisted value (obtainable from
userPreferencesRepositoryImpl.getSelectedServices() or your repository's
equivalent) before calling updateState { it.copy(isEditMode = false) } so
cancellation propagates correctly and the in-memory selections remain consistent
with storage.
🧹 Nitpick comments (3)
feature/home/src/commonMain/kotlin/org/mifos/mobile/feature/home/HomeScreen.kt (2)

330-331: Consider adding a semantic role for accessibility.

The Box uses bare .clickable { onClick() } without a semantic role. Screen readers won't announce this as an interactive element. The codebase already has a Modifier.onClick(...) extension (in ModifierExt.kt) that accepts a role parameter — using it with Role.Button would improve accessibility.

Suggested adjustment
         Box(
-            modifier = Modifier.clickable { onClick() },
+            modifier = Modifier.clickable(role = Role.Button) { onClick() },
         ) {

360-363: Hard-coded 4.dp — prefer a design token for consistency.

The rest of the file consistently uses DesignToken.* and KptTheme.spacing.* for spacing values. This hard-coded 4.dp is a minor inconsistency.

feature/home/src/commonMain/kotlin/org/mifos/mobile/feature/home/HomeViewModel.kt (1)

499-501: Remove the unused allServiceRoutes state field.

allServiceRoutes is populated in loadSavedServices (line 238) but is never read anywhere in the codebase—not in HomeScreen.kt or elsewhere in the ViewModel. Removing it will reduce state complexity without affecting functionality.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 Fix all issues with AI agents
In
`@feature/home/src/commonMain/kotlin/org/mifos/mobile/feature/home/HomeScreen.kt`:
- Around line 326-367: The clickable Box with role=Role.Button lacks an
accessible label because Image has contentDescription=null and the service title
Text sits outside the touch target; update the accessibility by either moving
the clickable modifier to the parent Column (so the onClick on Column encloses
both Image and the title Text) or add an explicit spoken label via the
clickable's onClickLabel and/or provide a non-null contentDescription on the
Image (use stringResource for the service title or a dedicated label string).
Locate the clickable modifier in the Box and the Image(contentDescription), and
change one of them so the button exposes a descriptive label to screen readers
(e.g., use Column.clickable or clickable(onClickLabel = stringResource(...))
and/or remove contentDescription=null and supply the title string).
🧹 Nitpick comments (3)
feature/home/src/commonMain/kotlin/org/mifos/mobile/feature/home/HomeScreen.kt (1)

426-439: Preview shows empty state instead of service cards.

HomeState defaults selectedServices to emptySet(), so the preview renders only the "no services" hint rather than the service grid. Populate selectedServices to make the preview representative:

             state = HomeState(
                 dialogState = null,
                 items = serviceCards,
                 uiState = HomeScreenState.Success,
+                selectedServices = serviceCards.map { it.route }.toSet(),
             ),
feature/home/src/commonMain/kotlin/org/mifos/mobile/feature/home/HomeViewModel.kt (2)

49-49: Misleading variable name: userPreferencesRepositoryImpl is typed as the interface.

The constructor parameter is typed UserPreferencesRepository (the interface), but named ...Impl. This is confusing — the Impl suffix conventionally indicates a concrete implementation. Consider renaming to userPreferencesRepository for consistency with the type.

-    private val userPreferencesRepositoryImpl: UserPreferencesRepository,
+    private val userPreferencesRepository: UserPreferencesRepository,

230-242: Extract allRoutes to avoid duplicated computation.

serviceCards.map { it.route }.toSet() appears both here (Line 235) and in the catch block of handleToggleEditMode (Line 262). Extract it once:

Suggested refactor
+    private val allServiceRoutes = serviceCards.map { it.route }.toSet()
+
     private fun loadSavedServices() {
-        val allRoutes = serviceCards.map { it.route }.toSet()
         val savedServices = userPreferencesRepositoryImpl.selectedServices
         updateState {
             it.copy(
-                selectedServices = savedServices ?: allRoutes,
+                selectedServices = savedServices ?: allServiceRoutes,
             )
         }
     }

And in handleToggleEditMode:

-                        selectedServices = lastSaved ?: serviceCards.map { it.route }.toSet(),
+                        selectedServices = lastSaved ?: allServiceRoutes,

@Aditya002500 Aditya002500 requested a review from biplab1 February 8, 2026 08:25
Copy link
Contributor

@biplab1 biplab1 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some minor changes suggested.

I have a suggestion: the user shouldn't be allowed to hide all the services. At least one service should remain visible.

You may discuss it with @therajanmaurya during the standup meeting.

Looks good to me. This can be merged.

@Aditya002500 Aditya002500 requested a review from biplab1 February 10, 2026 17:27
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

🤖 Fix all issues with AI agents
In
`@feature/home/src/commonMain/kotlin/org/mifos/mobile/feature/home/HomeViewModel.kt`:
- Around line 305-317: The toggle logic in handleToggleServiceSelection reads
state.selectedServices outside updateState, causing a race where rapid taps can
clobber toggles; move the computation of newSelection and visibleItems inside
the updateState lambda so you operate on the latest state snapshot (use the
lambda parameter e.g. it.selectedServices), then set selectedServices to the
computed newSelection and visibleItems to computeVisibleItems(selectedServices =
newSelection) within the same updateState call.
- Around line 63-65: The call to loadSavedServices() in HomeViewModel's init is
doing synchronous disk I/O via userPreferencesRepositoryImpl.selectedServices ->
getSelectedServicesDirectly() -> settings.getStringOrNull(); wrap the
loadSavedServices() invocation inside viewModelScope.launch(Dispatchers.IO) (or
launch { withContext(Dispatchers.IO) { ... } }) so the disk read runs off the
main thread, leaving saveSelectedServices as-is (it already uses
viewModelScope.launch); update references to loadSavedServices() in the init
block to call it from that coroutine context.
🧹 Nitpick comments (4)
feature/home/src/commonMain/kotlin/org/mifos/mobile/feature/home/HomeScreen.kt (2)

321-322: Default isSelected = true is a misleading default.

Defaulting to true means any call site that omits isSelected will render the card in a "selected" visual state. A safer default is false to avoid accidental visual misrepresentation.

Suggested fix
-    isSelected: Boolean = true,
+    isSelected: Boolean = false,

424-437: Preview shows all items as unselected.

selectedServices defaults to emptySet() in HomeState, so the preview renders every ServiceItemCard with isSelected = false (non-primary border/tint). Pass selectedServices explicitly to show a realistic preview:

Suggested fix
 HomeContent(
     state = HomeState(
         dialogState = null,
         items = serviceCards,
         uiState = HomeScreenState.Success,
+        selectedServices = serviceCards.map { it.route }.toSet(),
     ),
     onAction = {},
     modifier = Modifier,
 )
feature/home/src/commonMain/kotlin/org/mifos/mobile/feature/home/HomeViewModel.kt (2)

245-248: Same stale-state pattern in handleAmountVisible.

state.isAmountVisible is read outside the lambda's perspective — this uses the captured state property rather than the it parameter provided by MutableStateFlow.update. While this is pre-existing, the same pattern was introduced in the new toggle code. Consider fixing both:

 private fun handleAmountVisible() {
     updateState {
-        it.copy(isAmountVisible = !state.isAmountVisible)
+        it.copy(isAmountVisible = !it.isAmountVisible)
     }
 }

231-240: computeVisibleItems reads state by default — fragile when called inside updateState.

The default parameter values (state.isEditMode, state.selectedServices) capture a potentially stale snapshot when this function is invoked from within an updateState lambda. Callers inside updateState must always pass explicit parameters — but nothing enforces this. Consider removing the defaults to make callers explicit, or passing the full state:

Option: remove defaults to enforce explicit call sites
 private fun computeVisibleItems(
-    isEditMode: Boolean = state.isEditMode,
-    selectedServices: Set<String> = state.selectedServices,
+    isEditMode: Boolean,
+    selectedServices: Set<String>,
 ): ImmutableList<ServiceItem> {

@biplab1
Copy link
Contributor

biplab1 commented Feb 11, 2026

@mena-rizkalla Please review the updates.

@Aditya002500
Copy link
Contributor Author

@mena-rizkalla Please review the updates.

Sir i have not implemented anything new i just reverted to the original state

@mena-rizkalla
Copy link
Contributor

@mena-rizkalla Please review the updates.

Sir i have not implemented anything new i just reverted to the original state

Hi @Aditya002500 why you revert doing the filter in viewmodel ?
It was better because always try to follow the separation of concern principle where ui handle how to show the data while viewmodel handle what to show also this will make it easier for the developer who will write unit test for this class after you

@Aditya002500
Copy link
Contributor Author

@mena-rizkalla Please review the updates.

Sir i have not implemented anything new i just reverted to the original state

Hi @Aditya002500 why you revert doing the filter in viewmodel ? It was better because always try to follow the separation of concern principle where ui handle how to show the data while viewmodel handle what to show also this will make it easier for the developer who will write unit test for this class after you

Actually I was facing some issues in the implementation, and there is some stuff to discuss with Rajan Sir , so if another fix is recommended by sir , I will implement this together

Copy link
Contributor

@mena-rizkalla mena-rizkalla left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!
Moving the filtering logic to the ViewModel is a solid improvement and ensures a single source of truth for the UI state.

@biplab1 biplab1 merged commit 836e096 into openMF:development Feb 12, 2026
8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants